Batch 4C pt3: deployer serving-path gate + quiesce-timeout backout - #195
Conversation
Increment 4 of Batch 4C (docs/PLAN_4c_renderers.md). Two kayak-deploy.sh changes. Serving-path gate (when SERVING_CUTOVER=yes, before any mutation) — the check deferred from PR #190/#192 that the host is FULLY cut over, not half: - OUTPUT_DIR: every re-pointed unit that pins OUTPUT_DIR (the pipeline drop-in) must equal $KAYAK_DOCROOT, else the hourly pipeline builds a tree nobody serves — a silent half-cutover the symlink switch can't catch. Read via `systemctl show -p Environment` (Environment= drop-ins, not the EnvironmentFile, so pre-cutover units have none and skip). - nginx / FPM (knob-gated on KAYAK_NGINX_DOCROOT_CONF / KAYAK_FPM_POOL, which the 4C runbook sets): nginx roots at $KAYAK_DOCROOT, the ACME `root /var/www/certbot;` survives (PR #194 review #2 — a blanket root-sed would break renewal), and the FPM open_basedir leads with $KAYAK_DOCROOT. Unset ⇒ warn+skip (can't guess the paths) rather than silently pass. Quiesce-timeout backout (the deploy_quiesce_timeout_followup fix): the drain timeout's explicit `exit 1` doesn't fire the ERR trap even under -E, so a consumer that wouldn't drain left the host in maintenance with consumers stopped and no rollback (nothing mutated, but the site stayed down). It now restarts consumers + clears maintenance before exiting. Drain bound parameterized (KAYAK_DRAIN_TIMEOUT/_INTERVAL, prod defaults 120/2) so the backout is slow-testable in ~2 s. Tests: a half-cutover gate test (OUTPUT_DIR mismatch, nginx legacy root, clobbered certbot root, open_basedir missing the docroot — each refused pre-mutation) and a quiesce-timeout backout test. Existing 5 slow activation tests unchanged and green (new checks skip when OUTPUT_DIR/knobs absent). Full slow suite 7 passed; non-slow 1761 passed; ruff/format/shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial review — PR #195 (Batch 4C pt3: serving-path gate + quiesce-timeout backout)Reviewed on the live host, with What's right
1. (Medium)
|
…usivity
Four review findings on the serving-path gate, all closed:
1. (Medium) FAIL-CLOSED on the serving knobs. SERVING_CUTOVER=yes is set only by
the same 4C runbook that sets KAYAK_NGINX_DOCROOT_CONF/KAYAK_FPM_POOL, so an
unset knob under cutover is now an error, not a warn+skip — warn-skip let
exactly the nginx-side half-cutover (consumers re-pointed, web layer still on
the old docroot) sail through, which is the gate's reason to exist.
--stage-only exits before this gate, so it is unaffected.
2. (Low–Medium) EXCLUSIVE nginx root, not presence-only. Every `root` must be the
docroot or the ACME /var/www/certbot root — a leftover legacy
`root /home/pat/public_html;` (an apply that appended instead of replacing)
passed the presence checks yet nginx serves the LAST root.
3. (Low) Anchored the FPM open_basedir grep at line start so a `;`-commented line
can't satisfy it (mirrors the nginx check).
4. (Nits) $KAYAK_DOCROOT is now regex-escaped before interpolation into the grep
patterns — negated-class `sed 's#[^[:alnum:]_/-]#\&#g'`, portable across GNU +
BSD sed (the metachar-list form trips BSD, which the local slow tests use). The
hardcoded /var/www/certbot carries a note for genericization (step 7).
Plus a bug the new fail-closed test caught in the fix itself: `: "${VAR:?}"`
returned exit 0 because the cleanup EXIT trap masks a :? expansion-error exit
(bash exits with the trap's `return 0`). Switched both checks to explicit
`exit 1`, which is preserved across the trap (verified; bare `set -e` failures are
too — the two pre-existing :? fire before the EXIT trap is armed, so unaffected).
Tests: the gate test gains exclusive-root, fail-closed (unset knobs), and
commented-open_basedir cases; the five activation tests now supply the required
serving knobs via a shared `_serving_knobs` helper. Full slow suite 7 passed;
shellcheck clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — #1 and #2 are exactly the kind of gap that quietly defeats a safety gate, so I closed all of it. (Slow suite re-running locally now with the fail-closed change wired through every activation test; will confirm green.) 1 (Medium) — fail-closed on the serving knobs. Done: under 2 (Low–Medium) — exclusive, not presence-only. The nginx check now asserts every 3 (Low) — FPM anchor. Anchored the 4 (Nits). (a) Net: the gate now fails closed, verifies the docroot is served exclusively, and is metachar-safe. The warn-skip path is gone (so the "untested warn-skip" concern is moot). I'll post the slow-suite result when it finishes. Bonus finding while testing #1: the fail-closed check initially used |
Batch 4C — increment 4: deployer serving-path gate + the quiesce-timeout rollback fix
Two
deploy/kayak-deploy.shchanges that close the deferred serving-path verification and a sibling of the errtrace rollback bug. Both gated/tested so the existing activation paths are unaffected.Serving-path gate (when
SERVING_CUTOVER=yes, before any mutation)The deferred-from-#190/#192 check that the host is fully cut over, not half:
OUTPUT_DIR(consumer build target): for every re-pointed unit whose drop-in pinsOUTPUT_DIR(the pipeline), verify it equals$KAYAK_DOCROOT— else the hourly pipeline builds into a tree nobody serves, a silent half-cutover the symlink switch can't catch. Read viasystemctl show -p Environment(reflectsEnvironment=drop-ins, not theEnvironmentFile, so pre-cutover units have none and skip).KAYAK_NGINX_DOCROOT_CONF/KAYAK_FPM_POOLare set (the 4C runbook sets them), verify nginx roots at$KAYAK_DOCROOT, the ACMEroot /var/www/certbot;survives (PR Batch 4C pt2:levels render-serving(nginx root + FPM open_basedir) #194 review Reduce DB bloat, improve lock contention, add gauge metadata cache #2 — a blanket root-sed would break renewal), and the FPMopen_basedirleads with$KAYAK_DOCROOT. Unset ⇒ the gate warns and skips (it can't guess the paths) rather than silently passing.Quiesce-timeout backout (the [[deploy_quiesce_timeout_followup]] fix)
The drain-timeout
exit 1is an explicit exit, which doesn't fire theERRtrap even under-E— so a consumer that won't drain left the host in maintenance with consumers stopped and no rollback (nothing had mutated, but the site stayed down until an operator cleared it). It now restarts consumers + clears maintenance before theexit. The drain bound is parameterized (KAYAK_DRAIN_TIMEOUT/_INTERVAL, prod defaults 120/2) so the backout is slow-testable in ~2 s instead of 120.Tests
test_serving_path_gate_refuses_half_cutover— one full activation that passes the gate (and stages a release the refusal cases reuse), then four refusals:OUTPUT_DIRmismatch, nginx rooting the legacy docroot, a clobbered certbot ACME root, and anopen_basedirmissing the docroot. Asserts none mutated the DB.test_quiesce_timeout_backs_out_maintenance—is-activenever drains → timeout → maintenance cleared, consumers restarted, DB untouched.OUTPUT_DIR/knobs are absent). Full slow suite 7 passed; non-slow 1761 passed; ruff/format/shellcheck clean.Next: increment 5 (derive
KAYAK_UNITS/KAYAK_HOST_UNITSfrom the installed timers — D-CONSUMER), then the runbook §5 rewrite, then the VM-rehearsed live cutover.🤖 Generated with Claude Code